Skip to content

Conversation

@albertzaharovits
Copy link
Contributor

@albertzaharovits albertzaharovits commented Jun 26, 2025

This fixes a race condition in the test scenario, between the merge scheduler closing and the merge task being scheduled to run. The test scenario expects that the merge task runs when the scheduler is closed. If the merge scheduler is closed before the merge task is scheduled, the merge task will instead be scheduled as aborted.

Fixes: #125236

@albertzaharovits albertzaharovits self-assigned this Jun 26, 2025
@albertzaharovits albertzaharovits added >test Issues or PRs that are addressing/adding tests :Distributed Indexing/Engine Anything around managing Lucene and the Translog in an open shard. v8.19.0 v9.1.0 v9.0.4 v9.2.0 labels Jun 26, 2025
@elasticsearchmachine elasticsearchmachine added the Team:Distributed Indexing Meta label for Distributed Indexing team label Jun 26, 2025
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-distributed-indexing (Team:Distributed Indexing)

Copy link
Contributor

@henningandersen henningandersen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, though it looks like this PR contains excess changes?

muted-tests.yml Outdated
- class: org.elasticsearch.packaging.test.DockerTests
method: test081SymlinksAreFollowedWithEnvironmentVariableFiles
issue: https://github.com/elastic/elasticsearch/issues/128867
- class: org.elasticsearch.index.engine.ThreadPoolMergeExecutorServiceDiskSpaceTests
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks out-of-place for this PR?

aFileStore.totalSpace = randomLongBetween(1L, 100L);
bFileStore.usableSpace = randomLongBetween(1L, 100L);
bFileStore.totalSpace = randomLongBetween(1L, 100L);
long aUsableSpace;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This also looks like it belongs somewhere else? Maybe I reviewed this part in another PR already, looks familiar but did not check. Maybe once that is merged, this change disappears?

@albertzaharovits
Copy link
Contributor Author

Looks good, though it looks like this PR contains excess changes?

Oh boy, I've mixed up branches, I've got several things in-flight. Will address momentarily.

This reverts commit 47a3055.
@albertzaharovits
Copy link
Contributor Author

@henningandersen I've reverted the stray commit on this PR branch.

Copy link
Contributor

@henningandersen henningandersen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@albertzaharovits albertzaharovits added auto-merge-without-approval Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) auto-backport Automatically create backport pull requests when merged labels Jun 26, 2025
@albertzaharovits
Copy link
Contributor Author

@elasticsearchmachine run elasticsearch-ci/part-3

@elasticsearchmachine elasticsearchmachine merged commit 0a77bdf into elastic:main Jun 26, 2025
32 checks passed
@albertzaharovits albertzaharovits deleted the fix-125236 branch June 26, 2025 16:06
@elasticsearchmachine
Copy link
Collaborator

💔 Backport failed

Status Branch Result
8.19 Commit could not be cherrypicked due to conflicts
9.0 Commit could not be cherrypicked due to conflicts

You can use sqren/backport to manually backport by running backport --upstream elastic/elasticsearch --pr 130078

albertzaharovits added a commit to albertzaharovits/elasticsearch that referenced this pull request Jun 26, 2025
…rge (elastic#130078)

This fixes a race condition in the test scenario, between the merge
scheduler closing and the merge task being scheduled to run. The test
scenario expects that the merge task runs when the scheduler is closed.
If the merge scheduler is closed before the merge task is scheduled, the
merge task will instead be scheduled as aborted.

Fixes: elastic#125236
elasticsearchmachine pushed a commit that referenced this pull request Jun 29, 2025
…rge (#130078) (#130132)

This fixes a race condition in the test scenario, between the merge
scheduler closing and the merge task being scheduled to run. The test
scenario expects that the merge task runs when the scheduler is closed.
If the merge scheduler is closed before the merge task is scheduled, the
merge task will instead be scheduled as aborted.

Fixes: #125236
albertzaharovits added a commit to albertzaharovits/elasticsearch that referenced this pull request Jul 3, 2025
…rge (elastic#130078)

This fixes a race condition in the test scenario, between the merge
scheduler closing and the merge task being scheduled to run. The test
scenario expects that the merge task runs when the scheduler is closed.
If the merge scheduler is closed before the merge task is scheduled, the
merge task will instead be scheduled as aborted.

Fixes: elastic#125236
elasticsearchmachine pushed a commit that referenced this pull request Jul 3, 2025
…rge (#130078) (#130529)

This fixes a race condition in the test scenario, between the merge
scheduler closing and the merge task being scheduled to run. The test
scenario expects that the merge task runs when the scheduler is closed.
If the merge scheduler is closed before the merge task is scheduled, the
merge task will instead be scheduled as aborted.

Fixes: #125236
mridula-s109 pushed a commit to mridula-s109/elasticsearch that referenced this pull request Jul 3, 2025
…rge (elastic#130078)

This fixes a race condition in the test scenario, between the merge
scheduler closing and the merge task being scheduled to run. The test
scenario expects that the merge task runs when the scheduler is closed.
If the merge scheduler is closed before the merge task is scheduled, the
merge task will instead be scheduled as aborted.

Fixes: elastic#125236
albertzaharovits added a commit to albertzaharovits/elasticsearch that referenced this pull request Jul 3, 2025
…rge (elastic#130078)

This fixes a race condition in the test scenario, between the merge
scheduler closing and the merge task being scheduled to run. The test
scenario expects that the merge task runs when the scheduler is closed.
If the merge scheduler is closed before the merge task is scheduled, the
merge task will instead be scheduled as aborted.

Fixes: elastic#125236
elasticsearchmachine pushed a commit that referenced this pull request Jul 4, 2025
…rge (#130078) (#130565)

This fixes a race condition in the test scenario, between the merge
scheduler closing and the merge task being scheduled to run. The test
scenario expects that the merge task runs when the scheduler is closed.
If the merge scheduler is closed before the merge task is scheduled, the
merge task will instead be scheduled as aborted.

Fixes: #125236
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

auto-backport Automatically create backport pull requests when merged auto-merge-without-approval Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) :Distributed Indexing/Engine Anything around managing Lucene and the Translog in an open shard. Team:Distributed Indexing Meta label for Distributed Indexing team >test Issues or PRs that are addressing/adding tests v8.19.0 v9.0.4 v9.1.1 v9.2.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[CI] ThreadPoolMergeSchedulerTests testSchedulerCloseWaitsForRunningMerge failing

3 participants